Skip to content

Conversation

@Andeshog
Copy link
Contributor

  • Renamed test folders for more clarity
  • Extended the C++ library
  • More fitting namespaces

Next thing to do after this PR is merged is to refactor the code in vortex-auv to use this library and enforce the new standard of using it.

@Andeshog Andeshog requested a review from kluge7 October 11, 2025 15:17
@codecov
Copy link

codecov bot commented Oct 11, 2025

Codecov Report

❌ Patch coverage is 79.25311% with 50 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.40%. Comparing base (f109e3b) to head (4ed8328).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
cpp_test/test_types.cpp 56.25% 0 Missing and 28 partials ⚠️
cpp_test/test_math.cpp 85.71% 0 Missing and 17 partials ⚠️
src/math.cpp 91.30% 2 Missing and 2 partials ⚠️
include/vortex/utils/math.hpp 50.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #10      +/-   ##
==========================================
- Coverage   90.54%   89.40%   -1.15%     
==========================================
  Files           6        6              
  Lines         423      500      +77     
  Branches       53      142      +89     
==========================================
+ Hits          383      447      +64     
+ Misses         26        4      -22     
- Partials       14       49      +35     
Flag Coverage Δ
unittests 89.40% <79.25%> (-1.15%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
include/vortex/utils/types.hpp 93.87% <100.00%> (ø)
py_test/test_utils.py 100.00% <100.00%> (ø)
include/vortex/utils/math.hpp 50.00% <50.00%> (ø)
src/math.cpp 91.30% <91.30%> (ø)
cpp_test/test_math.cpp 85.71% <85.71%> (ø)
cpp_test/test_types.cpp 56.25% <56.25%> (ø)

... and 3 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@kluge7 kluge7 requested a review from Copilot October 13, 2025 06:52
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR refactors the vortex_utils package to restructure C++ and Python utilities with improved organization, extended C++ functionality, and more fitting namespaces.

Key changes:

  • Restructured test folders from test/ and tests/ to cpp_test/ and py_test/ for clarity
  • Extended C++ library with new mathematical utilities and QoS profile definitions
  • Migrated from vortex_utils to vortex::utils namespace hierarchy with separate modules for math, types, and qos_profiles

Reviewed Changes

Copilot reviewed 16 out of 17 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
vortex_utils/qos_profiles.py Adds Python QoS profile utility functions
src/math.cpp New C++ math utilities replacing old cpp_utils.cpp
src/cpp_utils.cpp Removed old C++ utilities file
include/vortex/utils/*.hpp New header structure with separate math, types, and qos_profiles modules
cpp_test/ Restructured C++ tests with comprehensive coverage
py_test/ Restructured Python tests with updated file paths
CMakeLists.txt Updated build configuration for new structure
README.md Updated documentation reflecting new API

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

}
}

// Test that a quaternion with flipped signs is correctly convverted to euler
Copy link

Copilot AI Oct 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Corrected spelling of 'convverted' to 'converted'.

Suggested change
// Test that a quaternion with flipped signs is correctly convverted to euler
// Test that a quaternion with flipped signs is correctly converted to euler

Copilot uses AI. Check for mistakes.
Eigen::Quaterniond q{euler_to_quat(roll, pitch, yaw)};
Eigen::Vector4d result{q.x(), q.y(), q.z(), q.w()};
Eigen::Vector4d expected{0.0, 0.0, 0.0, 1.0};
for (int i = 0; i < 3; ++i) {
Copy link

Copilot AI Oct 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Loop should iterate through all 4 quaternion components (0 to 3), not just the first 3. The quaternion has x, y, z, w components.

Suggested change
for (int i = 0; i < 3; ++i) {
for (int i = 0; i < 4; ++i) {

Copilot uses AI. Check for mistakes.
Eigen::Quaterniond q{euler_to_quat(roll, pitch, yaw)};
Eigen::Vector4d result{q.x(), q.y(), q.z(), q.w()};
Eigen::Vector4d expected{0.479, 0.0, 0.0, 0.877};
for (int i = 0; i < 3; ++i) {
Copy link

Copilot AI Oct 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Loop should iterate through all 4 quaternion components (0 to 3), not just the first 3. The quaternion has x, y, z, w components.

Copilot uses AI. Check for mistakes.
Eigen::Quaterniond q{euler_to_quat(roll, pitch, yaw)};
Eigen::Vector4d result{q.x(), q.y(), q.z(), q.w()};
Eigen::Vector4d expected{0.0, 0.479, 0.0, 0.877};
for (int i = 0; i < 3; ++i) {
Copy link

Copilot AI Oct 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Loop should iterate through all 4 quaternion components (0 to 3), not just the first 3. The quaternion has x, y, z, w components.

Copilot uses AI. Check for mistakes.
Eigen::Quaterniond q{euler_to_quat(roll, pitch, yaw)};
Eigen::Vector4d result{q.x(), q.y(), q.z(), q.w()};
Eigen::Vector4d expected{0.0, 0.0, 0.479, 0.877};
for (int i = 0; i < 3; ++i) {
Copy link

Copilot AI Oct 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Loop should iterate through all 4 quaternion components (0 to 3), not just the first 3. The quaternion has x, y, z, w components.

Copilot uses AI. Check for mistakes.
Eigen::Quaterniond q{euler_to_quat(roll, pitch, yaw)};
Eigen::Vector4d result{q.x(), q.y(), q.z(), q.w()};
Eigen::Vector4d expected{0.1675, 0.5709, 0.1675, 0.786};
for (int i = 0; i < 3; ++i) {
Copy link

Copilot AI Oct 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Loop should iterate through all 4 quaternion components (0 to 3), not just the first 3. The quaternion has x, y, z, w components.

Copilot uses AI. Check for mistakes.
Copy link
Contributor

@kluge7 kluge7 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Andeshog Andeshog merged commit 979d564 into main Oct 13, 2025
5 checks passed
@Andeshog Andeshog deleted the refactor/vortex-utils-structure branch October 13, 2025 07:12
@github-actions
Copy link

🎉 This PR is included in version 1.4.3 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants